Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename dangerouslySetInnerHTML to dangerousInnerHTML #2257

Closed
wants to merge 1 commit into from
Closed

Rename dangerouslySetInnerHTML to dangerousInnerHTML #2257

wants to merge 1 commit into from

Conversation

syranide
Copy link
Contributor

From #1515 (comment)

Steps for upgrading existing codebases:

  1. Replace dangerouslySetInnerHTML with dangerousInnerHTML.

cc @zpao @sebmarkbage

@syranide
Copy link
Contributor Author

Note to self, rename #2258 after this is merged (there will be lots of conflicts).

@zpao
Copy link
Member

zpao commented Sep 30, 2014

This doesn't deprecate nicely. We should make dangerouslySetInnerHTML work for a release.

@syranide
Copy link
Contributor Author

@zpao Will add.

@sebmarkbage
Copy link
Collaborator

Yea I guess this is helpful because you get the error message an it is easy.

If this wasn't easy we could just require a codemod since it's such a unique name.

On Sep 30, 2014, at 3:13 PM, Andreas Svensson [email protected] wrote:

@zpao Will add.


Reply to this email directly or view it on GitHub.

@syranide syranide self-assigned this Sep 30, 2014
@syranide
Copy link
Contributor Author

@zpao I've re-added dangerouslySetInnerHTML with a depreciation warning.

Why don't we have a utility helper for emitting warnings once per component and (optionally) with the component name automatically prefixed? ... Should I put up a PR for something like it?

@sebmarkbage
Copy link
Collaborator

Ugh. We should've done this earlier. If we do it now, it's not worth the headache. Maybe if we intro innerHTML?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants